Skip to content

[sync] cli: 16 commits from Forge#1

Open
Snider wants to merge 19 commits intomainfrom
dev
Open

[sync] cli: 16 commits from Forge#1
Snider wants to merge 19 commits intomainfrom
dev

Conversation

@Snider
Copy link
Contributor

@Snider Snider commented Mar 17, 2026

Forge → GitHub Sync

Commits: 16
Files changed: 53

Large sync — may need splitting for CodeRabbit review.


Co-Authored-By: Virgil virgil@lethean.io

Summary by CodeRabbit

  • New Features

    • Added internationalization support for the CLI with custom locale registration capabilities.
    • Doctor command now displays localized messages.
  • Refactor

    • Reorganized core CLI structure for improved modularity and initialization flow.
    • Simplified logging interface.
  • Removals

    • Removed Go development tools (testing, coverage, QA, formatting, linting).
    • Removed module, plugin, service, and session management features.

Snider and others added 16 commits March 15, 2026 15:28
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Strip 21 blank imports that dragged go-ml, go-devops, go-build, etc.
into the CLI binary. core CLI should only have its own commands.
Fixes marketplace.NewInstaller call to match current signature.

Direct deps: core/go only (was: 22 forge packages).
Binary: 24MB (was: ~80MB). Cross-compiles with CGO_ENABLED=0.

Co-Authored-By: Virgil <virgil@lethean.io>
core/cli is now a pure library (pkg/cli). The binary moves to
cmd/core/ as a separate sub-module with its own go.mod.

Removed from binary: gocmd (→ lint/go-build), service (→ go-process),
session (→ go-session), module (→ go-scm), plugin (→ go-scm).
Removed from framework: go-crypt, workspace, daemon_cmd.

Root go.mod: 1 direct forge dep (core/go). Cross-compiles CGO_ENABLED=0.

Co-Authored-By: Virgil <virgil@lethean.io>
I18nService now lives in go-i18n as NewCoreService() — any binary can
use it without importing cli. Log convenience functions use go-log
directly. Removed LogService/NewLogService/daemon_cmd wrappers.

Root go.mod: 1 direct forge dep (core/go).

Co-Authored-By: Virgil <virgil@lethean.io>
Ecosystem imports (go-devops, go-build, go-scm, lint, go-crypt) added
back to cmd/core/main.go. Library go.mod stays clean (1 forge dep).
Binary gets full toolchain via workspace build.

Co-Authored-By: Virgil <virgil@lethean.io>
go-devops v0.0.3→v0.1.9, go-build v0.2.0→v0.2.3, lint v0.3.0→v0.3.2.
Resolves stale core/go/pkg/* import paths. Binary cross-compiles
with GOWORK=off CGO_ENABLED=0.

Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
- Added locales/en.json with 90 translation keys for doctor, pkg commands
- Main() embeds CLI locales automatically
- MainWithLocales() accepts additional FSSource for consuming binaries
- Ecosystem packages can ship their own locale files

Co-Authored-By: Virgil <virgil@lethean.io>
…ading

Both WithCommands() and RegisterCommands() now accept an optional
fs.FS for translations. The CLI collects them via RegisteredLocales()
and the i18n service loads them on startup.

Packages just pass their embed.FS — no i18n import needed:
  cli.RegisterCommands(AddDevCommands, locales.FS)

Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Commands now attach AFTER i18n service starts — translations resolve.
go-build imports commented out until kin-openapi dep conflict is fixed.

Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
commandService.applyI18n() walks registered commands and sets
Short/Long from cmd.{name}.short/long keys automatically. Downstream
packages no longer need to call i18n.T() for command descriptions —
the CLI Conclave handles it via service name derivation.

This is the Conclave pattern: services inside a sealed core.New()
auto-discover each other's capabilities via the lifecycle hooks.

Co-Authored-By: Virgil <virgil@lethean.io>
@Snider Snider marked this pull request as ready for review March 17, 2026 17:02
@Snider
Copy link
Contributor Author

Snider commented Mar 17, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Snider
Copy link
Contributor Author

Snider commented Mar 17, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Snider
Copy link
Contributor Author

Snider commented Mar 17, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

The changes restructure the CLI architecture by relocating the main entry point from the root to cmd/core/, removing entire command packages (gocmd, module, plugin, service, session), and introducing comprehensive internationalization infrastructure. The pkg/cli layer was simplified to directly support i18n at command registration with new locale APIs and supporting infrastructure.

Changes

Cohort / File(s) Summary
CLI Entry Point Relocation
main.go, cmd/core/main.go, cmd/core/go.mod
Removed root main.go and created new entry point at cmd/core/main.go that registers config, doctor, help, and pkg commands; added new go.mod for core module.
Removed Go Commands Package
cmd/gocmd/*
Deleted entire gocmd package including fmt, lint, fuzz, qa, test, cov, install, mod, work commands and all supporting logic (~1,300 lines removed).
Removed Module Commands Package
cmd/module/*
Deleted module command package including install, list, update, remove subcommands and module management workflow (~210 lines removed).
Removed Plugin Commands Package
cmd/plugin/*
Deleted plugin command package including install, list, info, update, remove subcommands and plugin registry/installer integration (~376 lines removed).
Removed Service Commands Package
cmd/service/cmd.go
Deleted service daemon command implementation with start, stop, list, restart subcommands and process/manifest management (~274 lines removed).
Removed Session Commands Package
cmd/session/cmd_session.go
Deleted session command ecosystem including list, replay, search subcommands and session utilities (~235 lines removed).
i18n/Localization Infrastructure
pkg/cli/app.go, pkg/cli/commands.go, pkg/cli/locales/en.json
Added embed-based locale loading, WithLocales/MainWithLocales APIs, command-level i18n application via applyI18n, RegisteredLocales tracking, and new English locale resource file with cmd.doctor and cmd.pkg translations.
Simplified i18n and Logging
pkg/cli/i18n.go, pkg/cli/log.go
Removed I18nService wrapper and complex logging facade; simplified to direct delegation to underlying i18n.T and log functions; updated import paths.
Removed Daemon Command Support
pkg/cli/daemon_cmd.go
Deleted AddDaemonCommand, DaemonCommandConfig, and daemon lifecycle helpers (start, stop, status, foreground).
Doctor Command Localization
cmd/core/doctor/cmd_commands.go, cmd/core/doctor/cmd_doctor.go
Moved i18n.T calls from cmd_doctor.go struct initialization to AddDoctorCommands function; command registration now occurs after localization is set.
Runtime and Utils Updates
pkg/cli/runtime.go, pkg/cli/utils.go
Moved command attachment to after Core startup to ensure i18n availability; changed GhAuthenticated logging from LogSecurity to LogWarn.
Minor Import Reformatting
cmd/core/pkgcmd/cmd_install.go
Reorganized import block grouping without semantic changes.
Dependency Updates
go.mod
Updated module dependencies including new core/go v0.3.1, updated i18n and logging versions, and adjusted indirect dependency versions across ecosystem.

Sequence Diagram

sequenceDiagram
    actor User
    participant App as App Entry
    participant CLI as CLI Runtime
    participant i18n as i18n Service
    participant Commands as Command Registry
    
    User->>App: Execute cmd/core
    activate App
    App->>CLI: Main() / MainWithLocales()
    activate CLI
    
    alt WithLocales provided
        CLI->>i18n: Load embedded + caller-supplied locales
        activate i18n
        i18n-->>CLI: i18n.NewCoreService(ExtraFS)
        deactivate i18n
    else No locales
        CLI->>i18n: Default i18n.NewCoreService()
        activate i18n
        i18n-->>CLI: Default service
        deactivate i18n
    end
    
    CLI->>CLI: core.ServiceStartup()
    Note over CLI: Core services initialized
    
    CLI->>Commands: RegisterCommands(config, doctor, help, pkg)
    activate Commands
    Commands->>Commands: Register with localeFS
    Commands-->>CLI: Commands registered
    deactivate Commands
    
    CLI->>CLI: applyI18n(root)
    Note over CLI: Apply i18n translations to<br/>cmd.*.short and cmd.*.long keys
    
    CLI->>CLI: Execute command
    User->>User: View localized output
    deactivate CLI
    deactivate App
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Poem

🐰 The warren restructures its burrow,

Commands consolidated, old paths follow,

Locales embedded where messages dwell,

In cmd/core the new stories tell! 🌍✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[sync] cli: 16 commits from Forge' directly describes the PR's primary purpose as a synchronization containing 16 commits from the Forge repository.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev
📝 Coding Plan
  • Generate coding plan for human review comments

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.3)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (4)
pkg/cli/i18n.go (1)

7-13: Update the T doc comment to match the new behavior.

The wrapper no longer talks to a CLI-local service or has a distinct fallback path; it now always delegates directly to the global i18n package. The current comment is misleading API documentation. As per coding guidelines, "Library code - ensure good API design and documentation".

📝 Suggested wording
-// T translates a key using the CLI's i18n service.
-// Falls back to the global i18n.T if CLI not initialised.
+// T translates a key using the global i18n package.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/i18n.go` around lines 7 - 13, Update the doc comment for the function
T in pkg/cli/i18n.go to reflect its current behavior: remove references to a
CLI-local service or a fallback and state that this wrapper always delegates
directly to the global i18n.T, passing through optional args; ensure the comment
mentions the signature (T(key string, args ...map[string]any) string) and that
it simply forwards to i18n.T.
pkg/cli/commands.go (1)

54-71: Derive i18n keys from the full command path.

key := "cmd." + cmd.Name() can only produce keys like cmd.install.short, but the bundled locale file already uses nested keys such as cmd.pkg.install.short. This top-level-only walk also skips child commands entirely, so subcommands can’t benefit from the new auto-i18n path. As per coding guidelines, "Library code - ensure good API design and documentation".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/commands.go` around lines 54 - 71, The current applyI18n uses only
cmd.Name() and root.Commands() so it builds flat keys like "cmd.install.short"
and skips subcommands; change it to derive keys from the full command path and
walk all commands recursively. For each cobra.Command use cmd.CommandPath()
(replace spaces with dots) and build key :=
"cmd."+strings.ReplaceAll(cmd.CommandPath(), " ", ".") instead of
"cmd."+cmd.Name(); also change the traversal to visit every command (e.g., use
root.VisitCommands(...) or a recursive loop over cmd.Commands()) so subcommands
receive i18n keys; keep the same Short/Long conditional logic but use the new
key variable.
cmd/core/go.mod (1)

3-3: Use go 1.25.0 with toolchain go1.26.0 unless this module requires Go 1.26-only features.

The go directive sets a hard minimum Go version (toolchains refuse to use modules declaring a newer version). Go 1.26 defaults new modules to go 1.25.0. If the goal is to build with Go 1.26 rather than require 1.26 syntax/APIs, use toolchain go1.26.0 to suggest the preferred compiler without forcing a higher minimum version.

♻️ Suggested change
-go 1.26.0
+go 1.25.0
+toolchain go1.26.0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/core/go.mod` at line 3, Replace the module's hard minimum Go version
declared by the line "go 1.26.0" with "go 1.25" and add a toolchain directive
"toolchain go1.26.0" so the build prefers Go 1.26 without forcing it; update the
existing "go 1.26.0" directive (referenced as the go directive) to "go 1.25" and
insert the toolchain directive immediately after it, unless this module actually
requires Go 1.26-only APIs.
pkg/cli/app.go (1)

102-104: Document the expected locale directory structure for registered packages.

The loop assumes registered locale filesystems have translations at their root (Dir: "."). If a package embeds translations in a subdirectory (e.g., locales/), they won't be found.

Consider documenting this contract in the RegisterCommands API, or accept FSSource (with directory info) instead of bare fs.FS in the registration API for flexibility.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/app.go` around lines 102 - 104, The loop in app.go assumes
RegisteredLocales() returns fs.FS whose translations live at the FS root (Dir:
"."), which breaks if packages embed translations in a subdir; update the
registration API and consumers to carry directory info: change RegisterCommands
/ RegisteredLocales to register and return i18n.FSSource (or a struct with FS
and Dir) instead of plain fs.FS, update the loop in app.go to append each
returned i18n.FSSource directly (use its Dir rather than hardcoding "."), and
update all callers/tests accordingly; also add a short doc comment to the
RegisterCommands API explaining the expected directory contract if you prefer
the simpler doc-only option.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/cli/app.go`:
- Around line 72-74: The documentation comment "Exits with code 1 on error or
panic." is incorrectly placed above the LocaleSource type alias; move that
sentence into the doc block for MainWithLocales (or the Main function doc if
applicable) so it documents MainWithLocales's behavior instead of
LocaleSource—update the comment adjacent to the MainWithLocales declaration
(referencing MainWithLocales and LocaleSource) and remove it from above the
LocaleSource type alias.

In `@pkg/cli/commands.go`:
- Around line 110-115: RegisteredLocales currently returns the backing slice
registeredLocales after releasing registeredCommandsMu, exposing callers to
mutation/race; while holding the lock in RegisteredLocales(), allocate a new
slice and copy the contents of registeredLocales into it, then return that copy
so callers cannot mutate the internal registry or race with subsequent appends
(keep references to registeredCommandsMu, registeredLocales and
RegisteredLocales for locating the change).

In `@pkg/cli/locales/en.json`:
- Around line 22-23: The locale strings "issues" and "issues_error" are being
used by cmd/core/doctor/cmd_doctor.go for required-check failures but currently
contain issue-fetching copy and no Count placeholder; update
pkg/cli/locales/en.json to replace those entries (or add the correct keys that
cmd/core/doctor/cmd_doctor.go references) with copy reflecting
tooling/required-check failures and include the Count placeholder (e.g., "X
required tools missing: {{Count}}" and an error message that accepts Count), or
alternatively change cmd_doctor.go to reference the correct existing keys—ensure
the translation keys used in cmd_doctor.go and the JSON keys match and that the
string contains the Count interpolation.

In `@pkg/cli/utils.go`:
- Around line 24-28: The current logic calls LogWarn in both branches, causing
normal authenticated state to be logged as a warning; update the code that
checks the authenticated boolean (the branch using LogWarn and log.Username())
so that the authenticated path uses a non-warning level (e.g., LogInfo or no
log) and only the unauthenticated path calls LogWarn with the message "GitHub
CLI not authenticated" (referencing the LogWarn function, the authenticated
variable, and log.Username()).

---

Nitpick comments:
In `@cmd/core/go.mod`:
- Line 3: Replace the module's hard minimum Go version declared by the line "go
1.26.0" with "go 1.25" and add a toolchain directive "toolchain go1.26.0" so the
build prefers Go 1.26 without forcing it; update the existing "go 1.26.0"
directive (referenced as the go directive) to "go 1.25" and insert the toolchain
directive immediately after it, unless this module actually requires Go
1.26-only APIs.

In `@pkg/cli/app.go`:
- Around line 102-104: The loop in app.go assumes RegisteredLocales() returns
fs.FS whose translations live at the FS root (Dir: "."), which breaks if
packages embed translations in a subdir; update the registration API and
consumers to carry directory info: change RegisterCommands / RegisteredLocales
to register and return i18n.FSSource (or a struct with FS and Dir) instead of
plain fs.FS, update the loop in app.go to append each returned i18n.FSSource
directly (use its Dir rather than hardcoding "."), and update all callers/tests
accordingly; also add a short doc comment to the RegisterCommands API explaining
the expected directory contract if you prefer the simpler doc-only option.

In `@pkg/cli/commands.go`:
- Around line 54-71: The current applyI18n uses only cmd.Name() and
root.Commands() so it builds flat keys like "cmd.install.short" and skips
subcommands; change it to derive keys from the full command path and walk all
commands recursively. For each cobra.Command use cmd.CommandPath() (replace
spaces with dots) and build key := "cmd."+strings.ReplaceAll(cmd.CommandPath(),
" ", ".") instead of "cmd."+cmd.Name(); also change the traversal to visit every
command (e.g., use root.VisitCommands(...) or a recursive loop over
cmd.Commands()) so subcommands receive i18n keys; keep the same Short/Long
conditional logic but use the new key variable.

In `@pkg/cli/i18n.go`:
- Around line 7-13: Update the doc comment for the function T in pkg/cli/i18n.go
to reflect its current behavior: remove references to a CLI-local service or a
fallback and state that this wrapper always delegates directly to the global
i18n.T, passing through optional args; ensure the comment mentions the signature
(T(key string, args ...map[string]any) string) and that it simply forwards to
i18n.T.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 495159fa-5b22-47e1-b670-7c24a8117f92

📥 Commits

Reviewing files that changed from the base of the PR and between 09b851f and 0c1b74c.

⛔ Files ignored due to path filters (2)
  • cmd/core/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (51)
  • cmd/core/config/cmd.go
  • cmd/core/config/cmd_get.go
  • cmd/core/config/cmd_list.go
  • cmd/core/config/cmd_path.go
  • cmd/core/config/cmd_set.go
  • cmd/core/doctor/cmd_checks.go
  • cmd/core/doctor/cmd_commands.go
  • cmd/core/doctor/cmd_doctor.go
  • cmd/core/doctor/cmd_environment.go
  • cmd/core/doctor/cmd_install.go
  • cmd/core/go.mod
  • cmd/core/help/cmd.go
  • cmd/core/main.go
  • cmd/core/pkgcmd/cmd_commands.go
  • cmd/core/pkgcmd/cmd_install.go
  • cmd/core/pkgcmd/cmd_manage.go
  • cmd/core/pkgcmd/cmd_pkg.go
  • cmd/core/pkgcmd/cmd_remove.go
  • cmd/core/pkgcmd/cmd_remove_test.go
  • cmd/core/pkgcmd/cmd_search.go
  • cmd/gocmd/cmd_commands.go
  • cmd/gocmd/cmd_format.go
  • cmd/gocmd/cmd_fuzz.go
  • cmd/gocmd/cmd_go.go
  • cmd/gocmd/cmd_gotest.go
  • cmd/gocmd/cmd_qa.go
  • cmd/gocmd/cmd_tools.go
  • cmd/gocmd/coverage_test.go
  • cmd/module/cmd.go
  • cmd/module/cmd_install.go
  • cmd/module/cmd_list.go
  • cmd/module/cmd_remove.go
  • cmd/module/cmd_update.go
  • cmd/plugin/cmd.go
  • cmd/plugin/cmd_info.go
  • cmd/plugin/cmd_install.go
  • cmd/plugin/cmd_list.go
  • cmd/plugin/cmd_remove.go
  • cmd/plugin/cmd_update.go
  • cmd/service/cmd.go
  • cmd/session/cmd_session.go
  • go.mod
  • main.go
  • pkg/cli/app.go
  • pkg/cli/commands.go
  • pkg/cli/daemon_cmd.go
  • pkg/cli/i18n.go
  • pkg/cli/locales/en.json
  • pkg/cli/log.go
  • pkg/cli/runtime.go
  • pkg/cli/utils.go
💤 Files with no reviewable changes (23)
  • cmd/plugin/cmd_update.go
  • cmd/plugin/cmd_install.go
  • cmd/session/cmd_session.go
  • cmd/gocmd/cmd_fuzz.go
  • cmd/gocmd/cmd_gotest.go
  • cmd/module/cmd_install.go
  • cmd/module/cmd_update.go
  • cmd/plugin/cmd_info.go
  • cmd/gocmd/cmd_commands.go
  • cmd/gocmd/cmd_go.go
  • cmd/gocmd/cmd_tools.go
  • cmd/module/cmd_list.go
  • cmd/service/cmd.go
  • pkg/cli/daemon_cmd.go
  • cmd/plugin/cmd.go
  • cmd/module/cmd.go
  • cmd/module/cmd_remove.go
  • cmd/gocmd/coverage_test.go
  • cmd/gocmd/cmd_format.go
  • cmd/gocmd/cmd_qa.go
  • cmd/plugin/cmd_remove.go
  • main.go
  • cmd/plugin/cmd_list.go

Comment on lines 72 to +74
// Exits with code 1 on error or panic.
// LocaleSource pairs a filesystem with a directory for loading translations.
type LocaleSource = i18n.FSSource
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Misplaced documentation comment.

The comment "Exits with code 1 on error or panic." on line 73 appears to be a leftover from the old Main function's documentation block. It now incorrectly precedes the LocaleSource type definition, creating misleading documentation.

📝 Proposed fix
-// Exits with code 1 on error or panic.
 // LocaleSource pairs a filesystem with a directory for loading translations.
 type LocaleSource = i18n.FSSource

Move the "Exits with code 1 on error or panic." line to the MainWithLocales documentation block where it belongs.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Exits with code 1 on error or panic.
// LocaleSource pairs a filesystem with a directory for loading translations.
type LocaleSource = i18n.FSSource
// LocaleSource pairs a filesystem with a directory for loading translations.
type LocaleSource = i18n.FSSource
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/app.go` around lines 72 - 74, The documentation comment "Exits with
code 1 on error or panic." is incorrectly placed above the LocaleSource type
alias; move that sentence into the doc block for MainWithLocales (or the Main
function doc if applicable) so it documents MainWithLocales's behavior instead
of LocaleSource—update the comment adjacent to the MainWithLocales declaration
(referencing MainWithLocales and LocaleSource) and remove it from above the
LocaleSource type alias.

Comment on lines +110 to +115
// RegisteredLocales returns all locale filesystems registered by command packages.
func RegisteredLocales() []fs.FS {
registeredCommandsMu.Lock()
defer registeredCommandsMu.Unlock()
return registeredLocales
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Return a copy from RegisteredLocales().

This currently hands out the backing slice after the mutex is released. Any caller that mutates or retains it can corrupt the registry or race with later appends. As per coding guidelines, "Library code - ensure good API design and documentation".

🛡️ Suggested fix
 func RegisteredLocales() []fs.FS {
 	registeredCommandsMu.Lock()
 	defer registeredCommandsMu.Unlock()
-	return registeredLocales
+	out := make([]fs.FS, len(registeredLocales))
+	copy(out, registeredLocales)
+	return out
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// RegisteredLocales returns all locale filesystems registered by command packages.
func RegisteredLocales() []fs.FS {
registeredCommandsMu.Lock()
defer registeredCommandsMu.Unlock()
return registeredLocales
}
// RegisteredLocales returns all locale filesystems registered by command packages.
func RegisteredLocales() []fs.FS {
registeredCommandsMu.Lock()
defer registeredCommandsMu.Unlock()
out := make([]fs.FS, len(registeredLocales))
copy(out, registeredLocales)
return out
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/commands.go` around lines 110 - 115, RegisteredLocales currently
returns the backing slice registeredLocales after releasing
registeredCommandsMu, exposing callers to mutation/race; while holding the lock
in RegisteredLocales(), allocate a new slice and copy the contents of
registeredLocales into it, then return that copy so callers cannot mutate the
internal registry or race with subsequent appends (keep references to
registeredCommandsMu, registeredLocales and RegisteredLocales for locating the
change).

Comment on lines +22 to +23
"issues": "Open issues assigned to you:",
"issues_error": "Failed to fetch issues",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

These doctor failure strings are wired to the wrong code path.

In cmd/core/doctor/cmd_doctor.go, Lines 97-100 use these keys when required checks fail and pass Count, but the translations here read like an issue-fetching feature and ignore that placeholder. The command will print an unrelated summary and return an unrelated error whenever tooling is missing.

💬 Suggested copy
-			"issues": "Open issues assigned to you:",
-			"issues_error": "Failed to fetch issues",
+			"issues": "{{.Count}} required check(s) failed",
+			"issues_error": "{{.Count}} doctor check(s) failed",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"issues": "Open issues assigned to you:",
"issues_error": "Failed to fetch issues",
"issues": "{{.Count}} required check(s) failed",
"issues_error": "{{.Count}} doctor check(s) failed",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/locales/en.json` around lines 22 - 23, The locale strings "issues"
and "issues_error" are being used by cmd/core/doctor/cmd_doctor.go for
required-check failures but currently contain issue-fetching copy and no Count
placeholder; update pkg/cli/locales/en.json to replace those entries (or add the
correct keys that cmd/core/doctor/cmd_doctor.go references) with copy reflecting
tooling/required-check failures and include the Count placeholder (e.g., "X
required tools missing: {{Count}}" and an error message that accepts Count), or
alternatively change cmd_doctor.go to reference the correct existing keys—ensure
the translation keys used in cmd_doctor.go and the JSON keys match and that the
string contains the Count interpolation.

Comment on lines 24 to 28
if authenticated {
LogSecurity("GitHub CLI authenticated", "user", log.Username())
LogWarn("GitHub CLI authenticated", "user", log.Username())
} else {
LogSecurity("GitHub CLI not authenticated", "user", log.Username())
LogWarn("GitHub CLI not authenticated", "user", log.Username())
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify where GhAuthenticated is used and where auth-status warnings are emitted.

rg -n -C2 --type go '\bGhAuthenticated\s*\('
rg -n -C2 --type go 'LogWarn\("GitHub CLI (authenticated|not authenticated)"'
rg -n -C2 --type go 'func\s+LogWarn\s*\('

Repository: dAppCore/cli

Length of output: 1332


Only log warnings when GitHub CLI is unauthenticated.

The authenticated branch logs at warning level for a normal operational state. This creates alert noise and degrades signal quality for actual warnings. Move the logging to only the unauthenticated path (or use debug/info level for diagnostic logging).

Suggested change
-	if authenticated {
-		LogWarn("GitHub CLI authenticated", "user", log.Username())
-	} else {
+	if !authenticated {
 		LogWarn("GitHub CLI not authenticated", "user", log.Username())
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if authenticated {
LogSecurity("GitHub CLI authenticated", "user", log.Username())
LogWarn("GitHub CLI authenticated", "user", log.Username())
} else {
LogSecurity("GitHub CLI not authenticated", "user", log.Username())
LogWarn("GitHub CLI not authenticated", "user", log.Username())
}
if !authenticated {
LogWarn("GitHub CLI not authenticated", "user", log.Username())
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/utils.go` around lines 24 - 28, The current logic calls LogWarn in
both branches, causing normal authenticated state to be logged as a warning;
update the code that checks the authenticated boolean (the branch using LogWarn
and log.Username()) so that the authenticated path uses a non-warning level
(e.g., LogInfo or no log) and only the unauthenticated path calls LogWarn with
the message "GitHub CLI not authenticated" (referencing the LogWarn function,
the authenticated variable, and log.Username()).

Snider and others added 3 commits March 17, 2026 17:50
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant